Skip to content

Conversation

@joelanford
Copy link
Member

@joelanford joelanford commented Nov 19, 2025

Summary

This PR introduces support for SyntheticPermissions (experimental feature) and migrates OLM v1 from token-based authentication to ServiceAccount impersonation for improved security and simplified authentication management.

What Changed

1. CRD Generator Enhancements (Commit c08e679)

Extended the CRD generator to support channel-specific field requirements and descriptions:

  • Added tags to allow for differences between standard and experimental channels' required vs optional fields
  • Added opcon:standard:description tags for channel-specific documentation (complementing existing experimental descriptions)
  • Refactored opconTweaks functions to modify the parent schema's required fields list

These enhancements enable the following API changes.

2. Optional ServiceAccount in Experimental Channel (Commit 1094bc9)

Made the serviceAccount field optional in the experimental channel while keeping it required in the standard channel:

ClusterExtension API changes:

  • serviceAccount field uses new validation tags to be optional (experimental) vs required (standard)
  • Channel-specific descriptions explain the authentication behavior

Generated artifacts updated:

  • CRDs for both channels with appropriate field requirements
  • API reference documentation
  • All manifest files (standard, experimental, e2e variants)

3. SyntheticPermissions Feature Gate (Commit 189a05a)

Enabled the SyntheticPermissions feature gate and implemented the logic for synthetic user authentication. When serviceAccount.Name is empty in experimental:

  • Authenticates as synthetic user: olm:clusterextension:<clusterExtensionName>
  • Member of synthetic group: olm:clusterextensions

Feature gate:

  • Enabled in experimental helm values (helm/experimental.yaml)
  • Enabled in tilt configuration (helm/tilt.yaml)

RBAC permissions:

  • Added conditional RBAC rules for impersonating:
    • Any user (for synthetic users like olm:clusterextension:<clusterExtensionName>)
    • The olm:clusterextensions group
  • Rules only included when SyntheticPermissions feature gate is enabled

Implementation:

  • Updated SyntheticUserRestConfigMapper to check for empty SA name (instead of sentinel value)
  • Modified NewRBACPreAuthorizer to accept a userInfoMapper function
  • Implemented userInfoMapper in main.go that:
    • Returns synthetic user info when SA name is empty AND feature gate is enabled
    • Returns service account user info when SA name is specified
  • Updated tests to work with the new userInfoMapper pattern

Authentication behavior:

  • When ServiceAccount.Name is empty + SyntheticPermissions enabled: synthetic user impersonation
  • When ServiceAccount.Name is specified: service account impersonation

4. ServiceAccount Impersonation (Commit 324a970)

Replaced token-based authentication with ServiceAccount impersonation for all authentication in the standard and experimental channels:

Authentication layer changes:

  • Added ServiceAccountImpersonationConfig() function returning an ImpersonationConfig
  • Updated ServiceAccountRestConfigMapper() to use NewImpersonatingRoundTripper
  • Removed 285 lines of token management code:
    • tokengetter.go - Token lifecycle management
    • tokengetter_test.go - Token getter tests
    • tripper.go - Token injection round tripper

RBAC changes:

  • Changed from serviceaccounts/token create permission
  • To serviceaccounts impersonate permission

Controller simplifications:

  • Removed ServiceAccountNotFoundError handling (impersonation doesn't require SA to exist beforehand)
  • Removed authentication package import from controller

Tests updated:

  • Verify impersonation headers instead of token injection
  • Added tests for ServiceAccountImpersonationConfig
  • Updated controller tests that referenced TokenGetter

Benefits:

  • No token lifecycle management or expiration handling
  • No need to verify ServiceAccount exists before use
  • Simpler codebase with fewer moving parts
  • More secure - no tokens created or cached
  • Enhanced security - No longer requires creating highly privileged ServiceAccounts that could be mounted by workloads in the install namespace

Testing

  • Unit tests updated to verify impersonation behavior
  • Authorization tests updated for userInfoMapper pattern
  • CRD generation tests pass with new validation tags

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

Copilot AI review requested due to automatic review settings November 19, 2025 21:25
@joelanford joelanford requested a review from a team as a code owner November 19, 2025 21:25
@openshift-ci openshift-ci bot requested a review from ankitathomas November 19, 2025 21:25
@openshift-ci
Copy link

openshift-ci bot commented Nov 19, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign camilamacedo86, grokspawn, michaelryanpeter, thetechnick for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@netlify
Copy link

netlify bot commented Nov 19, 2025

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 324a970
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/6920cd6d1b32640008e693aa
😎 Deploy Preview https://deploy-preview-2351--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modernizes the authentication mechanism for ClusterExtension management by replacing token-based ServiceAccount authentication with Kubernetes impersonation. This change significantly improves the security posture by eliminating the need for actual ServiceAccount resources to exist in the cluster, reducing the risk of credential misuse.

Key changes:

  • Replaced TokenGetter/TokenInjectingRoundTripper with ServiceAccount impersonation using transport.NewImpersonatingRoundTripper
  • Removed the SyntheticPermissions feature gate and related synthetic authentication code
  • Simplified operator-controller RBAC to always use cluster-admin permissions (previously conditional on BoxcutterRuntime feature gate)
  • Updated all documentation and samples to reflect that ServiceAccount resources should NOT be created

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.

Show a summary per file
File Description
manifests/standard.yaml Updated CRD documentation to remove "must exist" requirements for ServiceAccounts; changed ClusterRole to grant full cluster-admin permissions with wildcards
manifests/standard-e2e.yaml Same CRD and ClusterRole updates as standard.yaml for e2e test environment
manifests/experimental.yaml Updated CRD documentation and ClusterRole permissions; changed ClusterRoleBinding to bind to operator-controller-manager-role instead of cluster-admin
manifests/experimental-e2e.yaml Same updates as experimental.yaml for e2e test environment
internal/operator-controller/features/features.go Removed SyntheticPermissions feature gate definition
internal/operator-controller/controllers/clusterextension_controller.go Removed special error handling for ServiceAccountNotFoundError
internal/operator-controller/authentication/tripper.go Deleted entire file - token injection round tripper no longer needed
internal/operator-controller/authentication/tokengetter_test.go Deleted test file for removed TokenGetter functionality
internal/operator-controller/authentication/tokengetter.go Deleted TokenGetter implementation and ServiceAccountNotFoundError
internal/operator-controller/authentication/synthetic_test.go Deleted test for removed synthetic user permissions
internal/operator-controller/authentication/synthetic.go Deleted synthetic user authentication implementation
internal/operator-controller/authentication/service_account_impersonation.go Added new function to create impersonation config for ServiceAccounts using proper system:serviceaccount username and groups
internal/operator-controller/action/restconfig_test.go Updated tests to verify impersonation headers instead of token injection; removed synthetic user tests
internal/operator-controller/action/restconfig.go Replaced token-based mapper with impersonation-based mapper; removed synthetic user logic
helm/olmv1/templates/rbac/clusterrolebinding-operator-controller-manager-rolebinding.yml Simplified to always use operator-controller-manager-role binding; removed BoxcutterRuntime feature gate conditional
helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml Changed from conditional with limited permissions to always-enabled with full cluster-admin wildcards; removed BoxcutterRuntime feature gate check
helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml Updated field documentation to remove "must exist" language for ServiceAccounts
helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml Same CRD documentation updates as standard variant
hack/demo/synthetic-user-cluster-admin-demo-script.sh Deleted demo script for removed synthetic user feature
docs/tutorials/install-extension.md Added prominent security warning explaining impersonation and why ServiceAccount resources should NOT be created
docs/howto/derive-service-account.md Updated title and content to focus on RBAC instead of ServiceAccount resources; added warnings against creating ServiceAccount resources
docs/draft/howto/use-synthetic-permissions.md Deleted entire documentation file for removed feature
docs/api-reference/olmv1-api-reference.md Updated API documentation to remove "must exist" requirements for ServiceAccounts
config/samples/olm_v1_clusterextension.yaml Replaced ServiceAccount resource with detailed comment explaining impersonation approach and security rationale
cmd/operator-controller/main.go Removed TokenGetter instantiation and SyntheticPermissions feature gate check; simplified to always use ServiceAccountRestConfigMapper
api/v1/clusterextension_types.go Updated Go documentation to remove "must exist" language and clarify that only RBAC configuration is needed

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@joelanford joelanford marked this pull request as draft November 20, 2025 22:36
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 20, 2025
Copilot AI review requested due to automatic review settings November 21, 2025 06:11
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2025
Copilot finished reviewing on behalf of joelanford November 21, 2025 06:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@joelanford joelanford changed the title ✨ Use ServiceAccount impersonation instead of token-based auth ✨ Add SyntheticPermissions support and migrate to ServiceAccount impersonation Nov 21, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 21, 2025
Copilot AI review requested due to automatic review settings November 21, 2025 15:55
Copilot finished reviewing on behalf of joelanford November 21, 2025 16:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


// ServiceAccountImpersonationConfig returns an ImpersonationConfig for impersonating a ServiceAccount.
// This allows authentication as a ServiceAccount without requiring an actual token.
func ServiceAccountImpersonationConfig(ext ocv1.ClusterExtension) transport.ImpersonationConfig {
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ServiceAccountImpersonationConfig does not handle the case where ext.Spec.ServiceAccount.Name is empty. In the experimental channel, the serviceAccount field is now optional, but this function will produce an invalid username format system:serviceaccount:namespace: (with a trailing colon and no name) when the service account name is empty.

This function should either:

  1. Check if the name is empty and return an error, or
  2. Be guarded by callers to ensure it's only called when the service account name is non-empty

Looking at the code in cmd/operator-controller/main.go lines 674-683, the caller does check for empty name before calling this function in the userInfoMapper, but ServiceAccountRestConfigMapper() in internal/operator-controller/action/restconfig.go (lines 38-50) calls this function without checking if the service account name is empty. This could lead to invalid impersonation attempts.

Suggested change
func ServiceAccountImpersonationConfig(ext ocv1.ClusterExtension) transport.ImpersonationConfig {
func ServiceAccountImpersonationConfig(ext ocv1.ClusterExtension) transport.ImpersonationConfig {
if ext.Spec.ServiceAccount.Name == "" {
panic("ServiceAccountImpersonationConfig: ServiceAccount name is empty, cannot impersonate an empty service account")
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the SyntheticPermissions feature gate is enabled alongside and the optionality of the ServiceAccount field, this should be all good.

However, I am not opposed to an explicit panic to make it easier for us to detect and avoid this issue. Thoughts from other maintainers?

@codecov
Copy link

codecov bot commented Nov 21, 2025

Codecov Report

❌ Patch coverage is 78.46154% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.03%. Comparing base (1355ff7) to head (324a970).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
cmd/operator-controller/main.go 11.11% 8 Missing ⚠️
hack/tools/crd-generator/main.go 90.47% 2 Missing and 2 partials ⚠️
internal/operator-controller/authorization/rbac.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2351      +/-   ##
==========================================
- Coverage   74.23%   74.03%   -0.20%     
==========================================
  Files          91       90       -1     
  Lines        7239     7199      -40     
==========================================
- Hits         5374     5330      -44     
- Misses       1433     1436       +3     
- Partials      432      433       +1     
Flag Coverage Δ
e2e 43.54% <39.13%> (-0.81%) ⬇️
experimental-e2e 49.01% <0.00%> (+0.53%) ⬆️
unit 58.49% <76.92%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ions

This commit enhances the CRD generator with the following capabilities:

- Support for <opcon:validation:Required> and <opcon:validation:Optional>
  tags to control field requirements in the generated CRDs
- Support for <opcon:standard:description> tags to provide
  channel-specific descriptions (complementing experimental descriptions)
- Refactored opconTweaks functions to accept parent schema, enabling
  modification of the required fields list

These changes provide more flexibility in defining CRD schemas with
channel-specific behavior and field requirements.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copilot AI review requested due to automatic review settings November 21, 2025 20:03
Copilot finished reviewing on behalf of joelanford November 21, 2025 20:06
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

If serviceAccount is specified, OLM will authenticate as that service account.
Otherwise, operator-controller will authenticate as:
- User: "olm:clusterextensions:<clusterExtensionName>"
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The synthetic user name format is incorrect. According to the implementation in internal/operator-controller/authentication/synthetic.go, the user name should be olm:clusterextension:<clusterExtensionName> (singular) not olm:clusterextensions:<clusterExtensionName> (plural). The group name correctly uses the plural form olm:clusterextensions.

Suggested change
- User: "olm:clusterextensions:<clusterExtensionName>"
- User: "olm:clusterextension:<clusterExtensionName>"

Copilot uses AI. Check for mistakes.
If serviceAccount is specified, OLM will authenticate as that service account.
Otherwise, operator-controller will authenticate as:
- User: "olm:clusterextensions:<clusterExtensionName>"
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The synthetic user name format is incorrect. According to the implementation in internal/operator-controller/authentication/synthetic.go, the user name should be olm:clusterextension:<clusterExtensionName> (singular) not olm:clusterextensions:<clusterExtensionName> (plural). The group name correctly uses the plural form olm:clusterextensions.

Suggested change
- User: "olm:clusterextensions:<clusterExtensionName>"
- User: "olm:clusterextension:<clusterExtensionName>"

Copilot uses AI. Check for mistakes.
If serviceAccount is specified, OLM will authenticate as that service account.
Otherwise, operator-controller will authenticate as:
- User: "olm:clusterextensions:<clusterExtensionName>"
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The synthetic user name format is incorrect. According to the implementation in internal/operator-controller/authentication/synthetic.go, the user name should be olm:clusterextension:<clusterExtensionName> (singular) not olm:clusterextensions:<clusterExtensionName> (plural). The group name correctly uses the plural form olm:clusterextensions.

Suggested change
- User: "olm:clusterextensions:<clusterExtensionName>"
- User: "olm:clusterextension:<clusterExtensionName>"

Copilot uses AI. Check for mistakes.
| `namespace` _string_ | namespace is a reference to a Kubernetes namespace.<br />This is the namespace in which the provided ServiceAccount must exist.<br />It also designates the default namespace where namespace-scoped resources<br />for the extension are applied to the cluster.<br />Some extensions may contain namespace-scoped resources to be applied in other namespaces.<br />This namespace must exist.<br />namespace is required, immutable, and follows the DNS label standard<br />as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-),<br />start and end with an alphanumeric character, and be no longer than 63 characters<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 63 <br />Required: \{\} <br /> |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />The ServiceAccount must exist in the namespace referenced in the spec.<br />serviceAccount is required. | | Required: \{\} <br /> |
| `namespace` _string_ | namespace is a reference to a Kubernetes namespace.<br />This designates the default namespace where namespace-scoped resources<br />for the extension are applied to the cluster.<br />Some extensions may contain namespace-scoped resources to be applied in other namespaces.<br />This namespace must exist.<br /><opcon:standard:description><br />This is also the namespace of the referenced service account.<br /></opcon:standard:description><br /><opcon:experimental:description><br />This is also the namespace of the referenced service account, if specified.<br /></opcon:experimental:description><br />namespace is required, immutable, and follows the DNS label standard<br />as defined in [RFC 1123]. It must contain only lowercase alphanumeric characters or hyphens (-),<br />start and end with an alphanumeric character, and be no longer than 63 characters<br />[RFC 1123]: https://tools.ietf.org/html/rfc1123 | | MaxLength: 63 <br />Required: \{\} <br /> |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br /><opcon:standard:description><br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />serviceAccount is required.<br /></opcon:standard:description><br /><opcon:experimental:description><br />If serviceAccount is specified, OLM will authenticate as that service account.<br />Otherwise, operator-controller will authenticate as:<br /> - User: "olm:clusterextensions:<clusterExtensionName>"<br /> - Group: "olm:clusterextensions"<br />The authenticated user must be configured with the necessary permissions to perform these interactions.<br /></opcon:experimental:description><br /><opcon:standard:validation:Required><br /><opcon:experimental:validation:Optional> | | |
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The synthetic user name format in the documentation is incorrect. The User should be olm:clusterextension:<clusterExtensionName> (singular) not olm:clusterextensions:<clusterExtensionName> (plural), according to the implementation in internal/operator-controller/authentication/synthetic.go. The group name correctly uses the plural form.

Suggested change
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br /><opcon:standard:description><br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />serviceAccount is required.<br /></opcon:standard:description><br /><opcon:experimental:description><br />If serviceAccount is specified, OLM will authenticate as that service account.<br />Otherwise, operator-controller will authenticate as:<br /> - User: "olm:clusterextensions:<clusterExtensionName>"<br /> - Group: "olm:clusterextensions"<br />The authenticated user must be configured with the necessary permissions to perform these interactions.<br /></opcon:experimental:description><br /><opcon:standard:validation:Required><br /><opcon:experimental:validation:Optional> | | |
| `serviceAccount` _[ServiceAccountReference](#serviceaccountreference)_ | serviceAccount is a reference to a ServiceAccount used to perform all interactions<br />with the cluster that are required to manage the extension.<br /><opcon:standard:description><br />The ServiceAccount must be configured with the necessary permissions to perform these interactions.<br />serviceAccount is required.<br /></opcon:standard:description><br /><opcon:experimental:description><br />If serviceAccount is specified, OLM will authenticate as that service account.<br />Otherwise, operator-controller will authenticate as:<br /> - User: "olm:clusterextension:<clusterExtensionName>"<br /> - Group: "olm:clusterextensions"<br />The authenticated user must be configured with the necessary permissions to perform these interactions.<br /></opcon:experimental:description><br /><opcon:standard:validation:Required><br /><opcon:experimental:validation:Optional> | | |

Copilot uses AI. Check for mistakes.
joelanford and others added 3 commits November 21, 2025 15:36
This commit updates the ClusterExtension API to make the serviceAccount
field optional in the experimental channel while keeping it required in
the standard channel.

Changes to ClusterExtension API:
- Added <opcon:standard:validation:Required> and
  <opcon:experimental:validation:Optional> tags to ServiceAccount field
- Added channel-specific descriptions for serviceAccount behavior:
  - Standard: Service account is required
  - Experimental: If omitted, authenticates as synthetic user
    "olmv1:clusterextensions:<clusterExtensionName>" with group
    "olmv1:clusterextensions"
- Added channel-specific descriptions for namespace field explaining
  the relationship with serviceAccount
- Updated ServiceAccountReference documentation to remove namespace
  requirement text
- Changed JSON tags to use omitzero for optional behavior

Generated artifacts updated:
- CRDs (standard and experimental channels)
- API reference documentation
- All manifest files

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
This commit enables the SyntheticPermissions feature to allow
ClusterExtensions to operate without specifying a ServiceAccount by
authenticating as synthetic users.

Feature gate changes:
- Enabled SyntheticPermissions in experimental channel (helm)
- Enabled SyntheticPermissions in tilt configuration

RBAC changes:
- Added conditional RBAC rules for impersonating users and the
  "olm:clusterextensions" group when SyntheticPermissions is enabled
- Kept existing serviceaccounts/token permissions for token-based auth

Logic changes:
- Updated SyntheticUserRestConfigMapper to check for empty SA name
  instead of synthetic-user sentinel value
- Modified NewRBACPreAuthorizer to accept a userInfoMapper function
- Implemented userInfoMapper in main.go to return synthetic user info
  when SA name is empty and feature gate is enabled, otherwise returns
  standard service account user info
- Updated authorization tests to work with new userInfoMapper pattern

With these changes:
- When ServiceAccount.Name is empty and SyntheticPermissions is enabled:
  authenticates as "olm:clusterextension:<name>" with group
  "olm:clusterextensions" using impersonation
- When ServiceAccount.Name is specified: uses existing token-based
  authentication (unchanged)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
This commit replaces token-based authentication with ServiceAccount
impersonation for better security and simpler token management.

Changes:

Authentication layer:
- Added ServiceAccountImpersonationConfig() function that returns an
  ImpersonationConfig for a ServiceAccount user
- Updated ServiceAccountRestConfigMapper() to use impersonation via
  NewImpersonatingRoundTripper instead of TokenInjectingRoundTripper
- Removed TokenGetter parameter from ServiceAccountRestConfigMapper
- Deleted tokengetter.go, tokengetter_test.go, and tripper.go as
  they are no longer needed

RBAC:
- Changed from serviceaccounts/token create to serviceaccounts
  impersonate permission

Controller:
- Removed ServiceAccountNotFoundError handling since impersonation
  doesn't require the ServiceAccount to exist beforehand
- Removed authentication package import from controller

Main setup:
- Removed TokenGetter initialization
- Updated userInfoMapper to use ServiceAccountImpersonationConfig

Tests:
- Updated tests to verify impersonation headers instead of token
  injection
- Added tests for ServiceAccountImpersonationConfig
- Updated controller tests that referenced TokenGetter

Benefits of impersonation over tokens:
- No need to manage token lifecycle or expiration
- No need to check if ServiceAccount exists before use
- Simpler code with fewer moving parts
- More secure as no tokens are created or cached
- More secure as it is no longer required to create highly privileged
  ServiceAccounts that could be used by workloads in the install
  namespace.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants